Skip to content

Modernize PHP test matrix, update PHPStan for 8.2+ compatibility#180

Merged
kadamwhite merged 21 commits intodevelopfrom
modernize-php-matrix
Apr 13, 2026
Merged

Modernize PHP test matrix, update PHPStan for 8.2+ compatibility#180
kadamwhite merged 21 commits intodevelopfrom
modernize-php-matrix

Conversation

@kadamwhite
Copy link
Copy Markdown
Contributor

On the branch from #178, PHPStan is failing locally due to outdated versions. This PR updates the PHP and WordPress minimum versions to modern targets and refreshes the PHP version matrices in CI jobs to cover non-EOL PHP.

Minimal code changes are included to resolve PHPStan issues, only one of which is a non-comment functional code change: since the $cache property of a WP_Object_Cache::$cache is initialized to [] it is not documented as nullable and the isset should not be required.

Removes deprecated PHP versions
Updates to latest stable WordPress
If phpstan trusts our annotation, it assumes the variable will always
exist and that the null check is therefore unneeded. Since we do not
control this global, we do need the check; therefore, we should tell
phpstan that it is nullable.
Solves error,
  function has parameter $additional with no value type specified in iterable type array.
@kadamwhite
Copy link
Copy Markdown
Contributor Author

Test failure cause:

In WP 6.6.1 (current on-dev version used in testing), the prepare_item_for_response method had

if ( in_array( 'roles', $fields, true ) ) {
    // Defensively call array_values() to ensure an array is returned.
    $data['roles'] = array_values( $user->roles );
}

But after updating in WP 6.9, a role guard was added:

if ( in_array( 'roles', $fields, true ) && ( current_user_can( 'list_users' ) || current_user_can( 'edit_user', $user->ID ) ) ) {
    // Defensively call array_values() to ensure an array is returned.
    $data['roles'] = array_values( $user->roles );
}

This was part of a security patch, changeset 60814.

goldenapples
goldenapples previously approved these changes Apr 7, 2026
Copy link
Copy Markdown
Contributor

@goldenapples goldenapples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes the issues with the build, but because this fix uncovered a breaking change in the plugin's functionality in recent versions of WP, I'd like to leave this open for discussion on how to handle that change.

@kadamwhite
Copy link
Copy Markdown
Contributor Author

Agree we don't need to rush this through. Opened #181 to break out the discussion of the cap check change and how we want to handle it. Looking for design input from longer-tenured contributors.

- vendor/szepeviktor/phpstan-wordpress/extension.neon
parameters:
level: max
level: 6
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this change was motivated by 50+ warnings which appeared after upgrading PHPStan, relating to PHPStan's assessment that many functions which returned the results of apply_filters now implicitly had the "mixed" return type. (Regardless of function return signature notations or PHPDoc.)

While this analysis is technically correct, the relevant functions had a return type specified on the function which would make malicious or misguided filter return values throw an error. To avoid excessive code changes in this PR I deemed it more prudent to lower the PHPStan level to 6 versus adding copious type coercion code, given that this return apply_filters() pattern is fairly core to how we develop on WordPress.

@kadamwhite
Copy link
Copy Markdown
Contributor Author

From @johnbillion (emphasis added):

[the need to verify that editors can create and assign co-authors is] why those tests use an editor already. I agree, let's get rid of that assertion on the roles element in the response

Done — reverted the prior change that switched the test case to use an admin, and instead removed the assertion about the GUEST_ROLE.

Copy link
Copy Markdown
Contributor

@roborourke roborourke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much @kadamwhite, trooper

@kadamwhite kadamwhite merged commit 00591da into develop Apr 13, 2026
6 checks passed
@kadamwhite kadamwhite deleted the modernize-php-matrix branch April 13, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants